--- title: Tech debt summary: CRITICAL and HIGH severity findings from analysis. status: draft owner: SAIF Doc Gen last_reviewed: 2026-09-22 --- # Tech debt This host carries no authorization construct of any kind across all six services, exposes an unconstrained arbitrary-path filesystem surface on `FileService`, logs sensitive `SecurityService` request detail unconditionally behind a dead kill switch, mutates a shared `private static` field on every request, and returns full server-side exception detail to every caller through a purely observational fault pipeline. ## CRITICAL | Finding | Component | Impact | Recommended action | | --- | --- | --- | --- | | No authorization construct exists anywhere in this host's code — every one of the 24 operations across all six services carries "None in code" for authorization, and access relies entirely on IIS authentication and network placement for the deployed variant | `FileService`, `PreferenceService`, `WebContextService`, `ImagingService`, `FaxService`, `SecurityService` | Any caller that can reach a route can invoke every operation on it, including the five destructive/unconstrained `FileService` operations | Add explicit authorization to at least the write- and delete-capable operations, or document and enforce the intended network-boundary protection | | `FileService` exposes five operations (`ReadFile`, `ReadEntireFile`, `WriteFile`, `DeleteFile`, `Length`) that act directly on a caller-supplied path via `System.IO` with no allow-list, no canonicalization, no containment check, and no authorization | `SAIF.Services.WCF.Services.FileService` | Any caller reaching the `FileService` route can read, overwrite, or delete any path the IIS application-pool identity can reach, including UNC paths; `includeExceptionDetailInFaults="true"` additionally returns resolved paths in filesystem exception detail on failure | Constrain accepted paths to an allow-listed root directory with `Path.GetFullPath` containment checks, and add caller-identity authorization before this surface is exposed further | | `SecurityService.LogRequestDetails` writes a JSON fragment of every operation's name and arguments — including `userId`, `employeeId`, `customerId`, `groupName`, and `role` — into the IIS log unconditionally on every call, and the `LOG_REQUEST_DETAILS` appSetting intended to gate it is read into a field but never consulted, so the kill switch has no effect | `SecurityService` | Sensitive identity arguments are written to the IIS log on every security operation with no way to disable it in configuration | Wire `_logRequestDetails` into an actual conditional check in `LogRequestDetails`, and reduce the logged fields to what is operationally necessary | | The fault pipeline is purely observational: `ErrorLogErrorHandler.HandleError` logs and returns `false`, `ProvideFault` is an empty method body, and `includeExceptionDetailInFaults="true"` means every unhandled exception reaches every caller with its full type, message, and stack trace | `ErrorLogErrorHandler`, `Web.Base.config` (``) | Any caller of any of the 24 operations can observe internal exception detail, including file paths and (for `FileService`/`PreferenceService`) caller-supplied values, on any unhandled failure | Set `includeExceptionDetailInFaults="false"` in the deployed configuration and introduce typed `[FaultContract]`s so callers receive a controlled failure shape instead of raw exception detail | ## HIGH | Finding | Component | Impact | Recommended action | | --- | --- | --- | --- | | `ldap_external` is a `private static` field mutated from an instance constructor on every request, and the `static` method `IsAuthorizedExternal` reads it; concurrent requests share and overwrite the same field | `SecurityService` | Under concurrent load, one request's resolved external LDAP host can be overwritten by another request's before the first request's authorization check runs — a cross-request shared-state hazard | Make the external LDAP host resolution a per-instance (or per-call, passed as a parameter) value rather than a mutable static field | | The six `[ServiceContract]` interfaces (`IFileService`, `IPreferenceService`, `IWebContextService`, `IImagingService`, `IFaxService`, `ISecurityService`) live outside this repository in `SAIF.Services.WCF.Contracts`, arriving only transitively via `SAIF.Framework.Core` with no direct reference and no version pin | All six service classes | The operation surface this host publishes can change without any corresponding change appearing in this repository's history | Add a direct, version-pinned reference to `SAIF.Services.WCF.Contracts` if it is intended to be a first-class dependency of this host | | `SAIFConnection.dll` 1.8.5.0 is vendored by `` outside `packages\`, with `False`, is not a NuGet package, and is not in `packages.config` — and no host source file references its types | `WCF.csproj` (`src/WCF/Libs/SAIF/SAIFConnection.dll`) | This binary cannot be restored, upgraded, or audited through the package pipeline; a rebuild depends on a file checked into the repository whose provenance is not tracked by any package manifest | Confirm whether this assembly is actually required at run time; remove the reference if it is not, or migrate it to a package if it is | | `ImagingService` is registered under the route name `Imaging` in both this host and `SAIF.Services.WCF32.Host`, but the two are different classes implementing different contracts (`SAIF.Services.WCF.Contracts.IImagingService` here vs. `SAIF.Services.WCF.Imaging.WCF32.IImagingService` there) on different platform targets (x64 vs. x86) | `SAIF.Services.WCF.Services.ImagingService`; also `SAIF.Services.WCF32.Host` (not documented — no pipeline reference, no `.nuspec` reference) | A change intended for one imaging surface can be mistaken for a change to the other; the two implementations can drift silently since neither references the other | Rename one of the two routes to remove the ambiguity, or document the split explicitly at the point of registration | | There is no dependency-injection container in this host; every collaborator across all six services is constructed with `new` against a concrete class inside a parameterless constructor or a per-call method, and `Autofac` 3.5.2 is referenced in `packages.config` but unused by any host source file | All six service classes | Collaborators cannot be substituted for testing or reconfigured without changing source; the unused `Autofac` reference adds confusion about whether a container is actually in use | Either remove the unused `Autofac` reference, or adopt it as the composition root if dependency substitution is needed | | `ImagingService.ThrowTimeoutException(int)` constructs a `FaultException` with `FaultCode("RequestTimeout")` but has no call site anywhere in the host — it is unreachable dead code | `SAIF.Services.WCF.Services.ImagingService` | The method suggests a timeout-fault contract exists for callers when none is actually ever raised, which can mislead a reader of the code into believing timeout handling exists on this path | Remove the dead method, or wire it into an actual timeout-detection path if one is intended | | `packages.config` is used exclusively for package management across the whole project, with no `PackageReference` migration path | `WCF.csproj`, `packages.config` | The project cannot take advantage of `PackageReference` features (transitive restore, lockfiles, floating versions) and remains on the older, more error-prone package-management model | Plan a migration to `PackageReference` when the host is next substantially touched | | `ErrorLogErrorHandler.HandleError` dereferences `error.TargetSite.DeclaringType` with no null guard; `TargetSite` is null for some exception shapes | `ErrorLogErrorHandler` | An exception whose `TargetSite` is null could cause the logging call itself to throw a secondary `NullReferenceException` inside the error handler, potentially masking the original failure's log entry | Add a null check around `error.TargetSite` before dereferencing `.DeclaringType` | ## Related documentation - [Index](index.md) - [Overview](overview.md) - [Architecture](architecture.md) - [Activation and hosting](activation-hosting.md) - [Operations](operations.md) - [Faults and error pipeline](faults.md) - [Authentication and authorization](authentication-authorization.md) - [Dependencies](dependencies.md) - [Configuration](configuration.md) - [Operations and processes](operations-processes.md) - [Business rules](business-rules.md) - [Data model](data-model.md) - [Operational runbook](operational-runbook.md) ## Sources - src/WCF/SAIF.Services.WCF.Host/Services/FileService.cs - src/WCF/SAIF.Services.WCF.Host/Services/Security/SecurityService.cs - src/WCF/SAIF.Services.WCF.Host/Services/ImagingService.cs - src/WCF/SAIF.Services.WCF.Host/Extensions/ErrorLogErrorHandler.cs - src/WCF/SAIF.Services.WCF.Host/Web.Base.config - src/WCF/SAIF.Services.WCF.Host/WCF.csproj - src/WCF/SAIF.Services.WCF.Host/packages.config